Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Navigation block: Fix submenu not opening on macOS Safari #62800

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Navigation block: Fix submenu not opening on macOS Safari #62800

merged 1 commit into from
Jun 24, 2024

Conversation

luisherranz
Copy link
Member

@luisherranz luisherranz commented Jun 24, 2024 •
edited
Loading

What?

This pull request fixes an issue in Safari Desktop (macOS) where submenus do not open upon clicking due to a change in #62160.

Why?

Right now (Gutenberg 18.6), the submenus that have the "Open on Click" option do not open on Safari Desktop.

How?

For now, I have simply reverted the change that causes the submenus to stop opening. I haven't investigated exactly why it happens yet.

It took us a while to figure out how to make the focus logic work properly in Safari since it functions completely differently than in Chrome and Firefox. That logic is also not as clean and straightforward as we would like:

I think for now we could leave this event handler as asynchronous to fix this bug and take a closer look at the Safari focus logic in a subsequent PR.

Testing Instructions

  • Add a menu with a submenu
  • Activate the option "Open on click"
  • Use Safari macOS
  • Test that the submenu opens on click
  • Test that the rest of features/accessibility work as usual

@luisherranz luisherranz added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block labels Jun 24, 2024
@luisherranz luisherranz self-assigned this Jun 24, 2024
@luisherranz luisherranz requested a review from ajitbohra as a code owner June 24, 2024 15:23
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 24, 2024 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: luisherranz <luisherranz@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@luisherranz
Copy link
Member Author

I think we need to add this fix to WP 6.6 and maybe even release a Gutenberg 18.6.1 version, don't we?

cc: @cbravobernal, @gziolo, @westonruter

@luisherranz luisherranz mentioned this pull request Jun 24, 2024
Closed
@github-actions GitHub Actions
Copy link

Flaky tests detected in 53d747b.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9648057681
📝 Reported issues:

@westonruter westonruter added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jun 24, 2024
westonruter
westonruter approved these changes Jun 24, 2024
View reviewed changes
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside: It would be great if the navigation block could start to leverage the Popover API since it is newly available in all browsers which would enable the menus to work even with JavaScript turned off entirely (cf. https://core.trac.wordpress.org/ticket/50182#comment:9)

@cbravobernal cbravobernal merged commit d97e3f7 into trunk Jun 24, 2024
69 checks passed
@cbravobernal cbravobernal deleted the fix/navigation-submenu-not-opening-on-safari-macos branch June 24, 2024 20:24
@cbravobernal cbravobernal added the Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release label Jun 24, 2024
@github-actions github-actions bot added this to the Gutenberg 18.7 milestone Jun 24, 2024
ellatrix pushed a commit that referenced this pull request Jun 25, 2024
@luisherranz @westonruter @ellatrix
Don't use async on submenu focus out (#62800)
f5ee0dd 
Co-authored-by: luisherranz <luisherranz@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
@ellatrix
Copy link
Member

I just cherry-picked this PR to the wp/6.6-rc-1 branch to get it included in the next release: f5ee0dd

@ellatrix ellatrix added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jun 25, 2024
cbravobernal pushed a commit that referenced this pull request Jun 25, 2024
@luisherranz @westonruter @cbravobernal
Don't use async on submenu focus out (#62800)
4114760 
Co-authored-by: luisherranz <luisherranz@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
@cbravobernal cbravobernal removed the Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release label Jun 25, 2024
ellatrix pushed a commit that referenced this pull request Jun 25, 2024
@luisherranz @westonruter @ellatrix
Don't use async on submenu focus out (#62800)
dd29a23 
Co-authored-by: luisherranz <luisherranz@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
@luisherranz
Copy link
Member Author

@cbravobernal just released GB 18.6.1 with this fix:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@westonruter westonruter westonruter approved these changes

@SantosGuillamot SantosGuillamot Awaiting requested review from SantosGuillamot

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees

@luisherranz luisherranz

Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Block] Navigation Affects the Navigation Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 18.6
Development

Successfully merging this pull request may close these issues.
4 participants
@luisherranz @ellatrix @westonruter @cbravobernal